fix(tui): scope model candidates to active provider by Hmbown · Pull Request #3428 · Hmbown/CodeWhale

Hmbown · 2026-06-23T01:11:39Z

Goal

Advance #3383 by making the main /model surfaces provider-scoped by default. A bare model string should no longer look like permission to switch providers.

Changes

added a reusable active-provider model candidate helper used by the picker and live slash-menu completions
removed cross-provider seeded/catalog rows from the main model picker section
removed cross-provider saved model rows from the main model picker section
changed /model <name> so saved models from other providers no longer silently emit SwitchProvider
kept active-provider saved models and current custom model reachability intact
added regression coverage for Together + saved OpenRouter rows not appearing as bare picker/completion choices

Verification

cargo test -p codewhale-tui --bin codewhale-tui --locked tui::model_picker
cargo test -p codewhale-tui --bin codewhale-tui --locked tui::widgets
cargo test -p codewhale-tui --bin codewhale-tui --locked visible_slash_model_completions_are_provider_scoped
cargo test -p codewhale-tui --bin codewhale-tui --locked commands::groups::core::core
cargo test -p codewhale-tui --bin codewhale-tui --locked model_inventory
cargo test -p codewhale-tui --bin codewhale-tui --locked route_resolver (0 matching tests)
python3 scripts/check-provider-registry.py
cargo fmt --all -- --check
git diff --check

Risks

Cross-provider route switching from the picker is removed from the main section in this slice. The explicit route-switch/search UX remains for v0.8.65: Cross-provider /model search using explicit route rows #3075/v0.8.65: Resolve every provider/model switch through a ReadyRouteCandidate #3384.
The lower-level slash_completion_hints(...) test helper still defaults to static provider rows; the live app path now passes the provider-scoped candidate projection from App.
This intentionally does not implement v0.8.65: Resolve every provider/model switch through a ReadyRouteCandidate #3384 ReadyRouteCandidate or v0.8.65: Provider-owned live catalogs and secret-free model cache #3385 live catalog/cache work.

Fixes #3383.

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

gemini-code-assist

Code Review

This pull request refactors the model picker and slash command completions to scope model choices strictly to the active provider, removing the automatic cross-provider switching behavior. The reviewer feedback recommends removing the unused _api_provider parameter from the newly introduced slash_completion_hints_with_model_candidates function and its callers to simplify the function signatures and reduce coupling.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-23T01:13:11Z

+    let model_candidates = model_completion_names_for_provider(api_provider)
+        .into_iter()
+        .map(str::to_string)
+        .collect::<Vec<_>>();
+    slash_completion_hints_with_model_candidates(
+        input,
+        limit,
+        cached_skills,
+        locale,
+        workspace,
+        api_provider,
+        &model_candidates,
+    )
+}
+
+pub(crate) fn slash_completion_hints_with_model_candidates(
+    input: &str,
+    limit: usize,
+    cached_skills: &[(String, String)],
+    locale: crate::localization::Locale,
+    workspace: Option<&std::path::Path>,
+    _api_provider: ApiProvider,
+    model_candidates: &[String],
 ) -> Vec<SlashMenuEntry> {


The _api_provider parameter in slash_completion_hints_with_model_candidates is unused. We can remove it to simplify the function signature and reduce coupling.

let model_candidates = model_completion_names_for_provider(api_provider) .into_iter() .map(str::to_string) .collect::<Vec<_>>(); slash_completion_hints_with_model_candidates( input, limit, cached_skills, locale, workspace, &model_candidates, ) } pub(crate) fn slash_completion_hints_with_model_candidates( input: &str, limit: usize, cached_skills: &[(String, String)], locale: crate::localization::Locale, workspace: Option<&std::path::Path>, model_candidates: &[String], ) -> Vec<SlashMenuEntry> {

gemini-code-assist · 2026-06-23T01:13:11Z

+    slash_completion_hints_with_model_candidates(
        &app.input,
        limit,
        &app.cached_skills,
        app.ui_locale,
        Some(&app.workspace),
        app.api_provider,
+        &model_candidates,
    )


Remove the unused app.api_provider argument from the call to slash_completion_hints_with_model_candidates.

slash_completion_hints_with_model_candidates( &app.input, limit, &app.cached_skills, app.ui_locale, Some(&app.workspace), &model_candidates, )

gemini-code-assist · 2026-06-23T01:13:11Z

+    let candidates = slash_completion_hints_with_model_candidates(
        &app.input,
        128,
        &app.cached_skills,
        app.ui_locale,
        Some(&app.workspace),
        app.api_provider,
+        &model_candidates,
    )


Remove the unused app.api_provider argument from the call to slash_completion_hints_with_model_candidates.

let candidates = slash_completion_hints_with_model_candidates( &app.input, 128, &app.cached_skills, app.ui_locale, Some(&app.workspace), &model_candidates, )

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Make the /model command, model picker main section, and slash completions stay inside the active provider scope. Cross-provider saved/catalog rows no longer appear as bare model choices or silently switch providers; explicit route-switch UX remains deferred to the route resolver/search work. Tests: cargo test -p codewhale-tui --bin codewhale-tui --locked tui::model_picker; cargo test -p codewhale-tui --bin codewhale-tui --locked tui::widgets; cargo test -p codewhale-tui --bin codewhale-tui --locked visible_slash_model_completions_are_provider_scoped; cargo test -p codewhale-tui --bin codewhale-tui --locked commands::groups::core::core; cargo test -p codewhale-tui --bin codewhale-tui --locked model_inventory; cargo test -p codewhale-tui --bin codewhale-tui --locked route_resolver; python3 scripts/check-provider-registry.py; cargo fmt --all -- --check; git diff --check

Keep the slash completion convenience wrapper test-only after the production slash menu path moved to explicit provider-scoped candidates. This avoids dead-code and unused-import failures under release/clippy builds while preserving the existing widget unit tests.

Remove the unused api_provider parameter from the provider-scoped slash completion helper after the live app path started passing explicit model candidates. Verified with: - cargo fmt --all -- --check - git diff --check - python3 scripts/check-provider-registry.py - cargo test -p codewhale-tui --bin codewhale-tui --locked tui::model_picker - cargo test -p codewhale-tui --bin codewhale-tui --locked tui::widgets - cargo test -p codewhale-tui --bin codewhale-tui --locked visible_slash_model_completions_are_provider_scoped - cargo test -p codewhale-tui --bin codewhale-tui --locked commands::groups::core::core - cargo test -p codewhale-tui --bin codewhale-tui --locked model_inventory - cargo test -p codewhale-tui --bin codewhale-tui --locked route_resolver (0 matching tests) - cargo test -p codewhale-tui --bin codewhale-tui --locked slash_completion_hints_model_provider_uses_provider_specific_ids - cargo clippy -p codewhale-tui --bin codewhale-tui --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or -A clippy::assertions_on_constants

claude · 2026-06-23T16:31:57Z

Claude finished @Hmbown's task in 3m 14s —— View job

PR Review: fix(tui): scope model candidates to active provider

Gather context and read changed files
Review correctness and logic
Review provider/model/route safety
Review reuse and simplification
Review test coverage
Post final review

Summary

Clean, focused implementation of #3383. The provider-scoped picker and slash-menu path are consistent, the removed saved_provider_model_match fallback is the right call, and the custom-model row preservation is intact. A few notes below:

Correctness

include_current_model flag semantics need a comment.
provider_scoped_model_ids_for_app is called with false from the picker and true from slash completions. The difference is subtle: false relies on show_custom_model_row in the picker to display the current non-standard model separately; true inlines it into the completions list so /model <custom> still appears as a candidate. This distinction isn't documented at the call sites or on the parameter, so it reads like a footgun for future maintainers.

model_picker.rs:383-388 — a short doc comment on provider_scoped_model_completion_ids and on the include_current_model parameter of the private fn would prevent confusion.

app.provider_models.get(app.api_provider.as_str()) in the scoped list.
The saved model for the active provider is appended without a validate_route check. This is fine because apply_model_picker_choice in ui.rs already validates before applying. The guard is still in place (the ui.rs diff confirms it was only a comment update). No action needed — just confirming the safety net is intact.

model command error message — the new message says "Expected auto or a model for the active provider. Common DeepSeek models: …" but doesn't hint at how to switch providers. That's intentional per the PR description (#3075/#3384 gap), but once the explicit route-switch UX lands it may be worth adding a short pointer here.

Provider/Model/Route Safety (EPIC #2608 invariant)

Looks good. The three code paths that could previously infer a silent provider switch are all addressed:

Path	Before	After
`/model <name>` for a cross-provider saved model	emitted `SwitchProvider`	returns `Invalid model` error
Main picker rows	included other-provider saved/catalog rows	restricted to active provider + "auto"
Slash `/model <name>` completions	used bare `api_provider` → all catalog rows	uses `provider_scoped_model_completion_ids(app)`

No code in this diff infers a provider switch from a model string prefix or freeform text. ✓

Reuse / Simplification

Gemini feedback already resolved. The _api_provider parameter the Gemini bot flagged was removed in commit 51fb515 fix(tui): trim slash completion helper signature. The current slash_completion_hints_with_model_candidates signature is clean: model_candidates: &[String], no provider arg.

slash_completion_hints is now #[cfg(test)] only. Good — the test-only wrapper keeps the old API for unit tests that don't build a full App, while the live path uses the more-specific projection.

push_model_id / push_model_row helpers are clean case-insensitive dedup utilities. No concerns.

Tests

Good regression coverage. A few observations:

try_autocomplete_slash_command provider-scoping is not directly tested. Both visible_slash_menu_entries and try_autocomplete_slash_command now call slash_completion_hints_with_model_candidates with the same model_candidates projection, so the underlying behavior is verified. Still, a dedicated test for the Tab-completion path (as opposed to the popup path) would make the coverage explicit. Low priority.
picker_exposes_saved_model_for_active_provider accesses view.model_rows and view.selected_model_idx directly (they're accessible because the test module is in-module). If these fields change visibility this test will catch it at compile time, so no latent risk.
visible_slash_model_completions_are_provider_scoped uses DEFAULT_TOGETHER_MODEL constant by name in the assertion — robust to renamed model IDs. ✓
Missing: auto-model path for completions. The condition include_current_model && !app.auto_model means auto-model doesn't append the current model to slash completions. There's no test asserting that app.auto_model = true leaves completions purely catalog-driven. Minor gap.

Minor nit

picker_model_hint returns "provider model" as the fallback hint for unknown model IDs (model_picker.rs:463). After this refactor the unknown IDs appearing in the picker are either the active-provider's saved model or the current custom model. The string "provider model" is accurate but a bit redundant now. Not worth a follow-up, just noting it.

Overall: ready to merge. The Gemini feedback was addressed before this review. The route-safety invariant is correctly enforced. No correctness bugs found.
· Branch

Document the include_current_model flag split between slash completions and the picker custom-model row. Verification: - cargo fmt --all -- --check - git diff --check - CARGO_TARGET_DIR=/Volumes/VIXinSSD/codewhale-target-v0865 cargo test -p codewhale-tui --bin codewhale-tui --locked tui::model_picker

claude · 2026-06-23T16:37:20Z

Claude finished @Hmbown's task in 3m 14s —— View job

Gather context and read changed files
Review correctness and logic
Review provider/model/route safety
Review reuse and simplification
Review test coverage
Post final review

PR Review: fix(tui): scope model candidates to active provider (re-review after sync)

This review covers the updated branch as of the four commits (03c1ad8 → b822d46). The Gemini bot's feedback was fully incorporated in 51fb515 and the previous documentation gap was closed in b822d46. Below is the final read.

Correctness

build_event provider filtering is correct for the active-provider saved-model path.
picker_exposes_saved_model_for_active_provider asserts provider == None in the emitted event. That's correct: build_event filters resolved_provider() with .filter(|p| *p != self.initial_provider), so selecting a model from the active provider emits None (no switch). The row still carries Some(XiaomiMimo) internally; it's the event that strips it when there's no change. ✓

push_model_id / push_model_row double deduplication is harmless.
Model IDs are deduplicated by case-insensitive string in push_model_id, then rows are deduplicated by (id, provider) in push_model_row. Since picker_model_rows_for_app assigns Some(app.api_provider) to every non-"auto" row, the second guard never fires. Not a bug — just a comment-free safety net. ✓

Minor nit — hardcoded literal vs constant in visible_slash_model_completions_are_provider_scoped:
The /model assertion uses a raw string:

assert!(names.contains(&"/model deepseek-ai/DeepSeek-V4-Pro"));

The model picker test uses the constant instead (crate::config::DEFAULT_TOGETHER_MODEL). Using the constant keeps the assertion refactor-safe:

assert!(names.contains(&format!("/model {}", crate::config::DEFAULT_TOGETHER_MODEL).as_str()));

Low priority — won't break anything unless the constant value is renamed.

include_current_model = true with a cross-provider app.model.
If the session model is, say, "gpt-4o" while api_provider = Together, it would appear in slash completions. Selecting it would then fail at normalize_model_name_for_provider in the model command, which is the correct safety net. Pre-existing possibility; not introduced here and not worsened.

Provider/Model/Route Safety (EPIC #2608)

All three leakage paths from the prior review remain cleanly closed:

Path	Status
`/model <name>` for cross-provider saved model	Returns `"Invalid model"` error, no `SwitchProvider` ✓
Main picker rows	Active provider + auto only ✓
`/model <name>` completions	`provider_scoped_model_completion_ids(app)` ✓

Important: deepseek-ai/DeepSeek-V4-Pro in the Together test is correct.
DEFAULT_TOGETHER_MODEL = "deepseek-ai/DeepSeek-V4-Pro" is a Together wire ID — the deepseek-ai/ prefix is a Together namespace, not a provider-switch hint. The test correctly asserts this model appears in Together completions while the OpenRouter saved model does not. ✓ No EPIC #2608 violation.

Reuse / Simplification

saved_provider_model_match / normalize_model_for_provider_selection fully removed with their import. Clean. ✓
slash_completion_hints correctly marked #[cfg(test)]; live path uses the parameterized slash_completion_hints_with_model_candidates. ✓
provider_scoped_model_completion_ids and provider_scoped_model_ids_for_app share one implementation with a single bool switch — good separation of the public/private API. ✓

Tests

Good coverage for the new behavior. Two remaining gaps noted for awareness (neither is a blocker):

try_autocomplete_slash_command (Tab-completion) not independently tested. Both visible_slash_menu_entries and try_autocomplete_slash_command now call slash_completion_hints_with_model_candidates with the same projection, so the behavior is indirectly verified. An explicit Tab-completion test would make the regression fence explicit.
auto_model = true + slash completions not tested. When auto_model = true, the include_current_model && !app.auto_model guard skips adding app.model. No test exercises this branch in the slash-completion context.

Overall: ready to merge. Gemini feedback addressed (51fb515), documentation gaps from the previous review addressed (b822d46), route-safety invariant correctly enforced, no correctness bugs found.
· Branch

greptile-apps Bot reviewed Jun 23, 2026

View reviewed changes

Hmbown mentioned this pull request Jun 23, 2026

v0.8.65: Provider-scoped model candidates for /model, picker, and slash completions #3383

Closed

gemini-code-assist Bot reviewed Jun 23, 2026

View reviewed changes

greptile-apps Bot reviewed Jun 23, 2026

View reviewed changes

Hmbown marked this pull request as ready for review June 23, 2026 05:56

greptile-apps Bot reviewed Jun 23, 2026

View reviewed changes

Hmbown added 3 commits June 23, 2026 09:21

Hmbown force-pushed the issue/3383-provider-scoped-model-candidates branch from 08df5a7 to 51fb515 Compare June 23, 2026 16:31

Hmbown merged commit d998c60 into main Jun 23, 2026
15 checks passed

Hmbown deleted the issue/3383-provider-scoped-model-candidates branch June 23, 2026 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tui): scope model candidates to active provider#3428

fix(tui): scope model candidates to active provider#3428
Hmbown merged 4 commits into
mainfrom
issue/3383-provider-scoped-model-candidates

Hmbown commented Jun 23, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

greptile-apps Bot left a comment

Uh oh!

claude Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Hmbown commented Jun 23, 2026

Goal

Changes

Verification

Risks

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: fix(tui): scope model candidates to active provider

Summary

Correctness

Provider/Model/Route Safety (EPIC #2608 invariant)

Reuse / Simplification

Tests

Minor nit

Uh oh!

claude Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: fix(tui): scope model candidates to active provider (re-review after sync)

Correctness

Provider/Model/Route Safety (EPIC #2608)

Reuse / Simplification

Tests

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Jun 23, 2026 •

edited

Loading

claude Bot commented Jun 23, 2026 •

edited

Loading